-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix(react): allow using enabled
when using useQuery().promise
#8501
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
View your CI Pipeline Execution ↗ for commit 5e9f958.
☁️ Nx Cloud last updated this comment at |
enabled
when using useQuery().promise
const observer = new QueryObserver(queryClient, { | ||
queryKey: key, | ||
enabled: false, | ||
queryFn: () => 'data', | ||
}) | ||
const results: Array<QueryObserverResult> = [] | ||
|
||
const success = pendingThenable<void>() | ||
|
||
const unsubscribe = observer.subscribe((result) => { | ||
results.push(result) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it should just have a rejected promise when enabled
is false
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree that’s how it should be 👍
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8501 +/- ##
===========================================
+ Coverage 46.25% 63.04% +16.78%
===========================================
Files 199 135 -64
Lines 7530 4835 -2695
Branches 1719 1358 -361
===========================================
- Hits 3483 3048 -435
+ Misses 3668 1543 -2125
+ Partials 379 244 -135 |
const observer = new QueryObserver(queryClient, { | ||
queryKey: key, | ||
enabled: false, | ||
queryFn: () => 'data', | ||
}) | ||
const results: Array<QueryObserverResult> = [] | ||
|
||
const success = pendingThenable<void>() | ||
|
||
const unsubscribe = observer.subscribe((result) => { | ||
results.push(result) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree that’s how it should be 👍
const shouldFetch = | ||
!state || | ||
(state.data === undefined && | ||
state.status === 'pending' && | ||
state.fetchStatus === 'idle') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would really want this to only trigger if there is no cache entry. As soon as you have a cache entry, no matter what state, the “thing” becomes observable by other components, making doing something “in render” not allowed by the rules of react.
I guess this was done to catch transitions from enabled:false
to enabled:true
?
Closes #8499